feat(otel): add actor lifecycle + scheduler duration metrics by krisztianfekete · Pull Request #514 · agent-substrate/substrate

Krisztian F (krisztianfekete) · 2026-07-24T09:22:24Z

This PR is the second slice of the platform-metrics split (#433).

It adds two duration histograms emitted by ateapi:

ate.actor.lifecycle.operation.duration: create/resume/suspend/pause/delete, labeled by operation, template, pool, sandbox class, and (on resume) snapshot kind
- I'd like to use this for user-facing latency for e.g. showing when suspended actors can serve requests again. The existing rpc.server.call.duration metric covers this, but the meaningful dimensions are missing, so it's not really actionable. Extending its labels with extra, domain-specific labels is an OTel anti-pattern, hence the new metric.
ate.scheduler.assignment.duration: worker-assignment step, labeled by outcome (assigned / no_free_worker / error) and pool
- I'd like to use this to alert on no free worker situation, and have proper SLOs via various percentiles for assigning latencies. There's no RPC around this, so it's not something existing RPC metrics cover.

Tested e2e on a local kind cluste where: both metrics reach the otel-system collector with the expected labels (resume shows snapshot_kind=golden, scheduler shows outcome=assigned, no error.type on success).

Tests pass
Appropriate changes to documentation are included in the PR

Eitan Yarmush (EItanya) · 2026-07-24T14:56:06Z


-func (s *Service) CreateActor(ctx context.Context, req *ateapipb.CreateActorRequest) (*ateapipb.Actor, error) {
-	if err := validateCreateActorRequest(req); err != nil {
+func (s *Service) CreateActor(ctx context.Context, req *ateapipb.CreateActorRequest) (created *ateapipb.Actor, err error) {


why are we swapping everything to be named return args, that's not a very common pattern. If you're trying to capture err can you just do var err error. I don't feel super duper strongly about this, but I personally find it a bit harder to read.

It's intentional, because with a var err error the defer would capture the pre-mapping error and also misqualify error.type. Would you say a comment would make this clearer?

Eitan Yarmush (EItanya) · 2026-07-24T14:59:43Z

+		s.instruments.recordLifecycleOp(ctx, ateattr.OperationDelete, start, err,
+			ateattr.TemplateNameKey.String(tmpl.GetActorTemplateName()),
+			ateattr.TemplateNamespaceKey.String(tmpl.GetActorTemplateNamespace()),
+		)


i think the behavior of this might be confusing. If this method returns before tmpl is assigned it may create a record with empty name, namespace. I think it's better to just use the incoming values even if they don't exist, what do you think?

Fair, although DeleteActorRequest is an ObjectRef so there's no template. I'll just drop template labels from delete entirely.

Dropped this one.

Eitan Yarmush (EItanya) · 2026-07-24T15:02:39Z

+// A nil *Instruments is a valid no-op, so call sites need no guard. Worker-count
+// is registered separately (RegisterWorkerCount): a callback-driven observable,
+// not a synchronous instrument.
+type Instruments struct {


Is there any reason this is public, seems like we could construct it locally rather than exporting it

It's exported because main.go builds it and passes it to NewService like e.g. RegisterWorkerCount, so this seemed like the consistent way.

Krisztian F (krisztianfekete) · 2026-07-31T08:41:57Z

cc. Da Huang (@git286), Julian Gutierrez Oschmann (@juli4n) for review. This has been up for a good while now, can you please review when you have a chance?

Zoe Zhao (zoez7) · 2026-07-31T21:06:29Z

Tim Bai (@baizhenyu) Jeff Luo (@JeffLuoo)

Jeff Luo (JeffLuoo) · 2026-08-04T14:30:44Z

+	// Recorded before the lock so lock contention still counts as an attempt; the
+	// incoming ctx stays valid where acquireActorLock returns nil on failure.
+	defer func() {
+		w.instruments.recordLifecycleOp(ctx, ateattr.OperationResume, start, err,
+			lifecycleOpAttrs(state.Actor, state.ActorTemplate, state.SnapshotKind)...)
+	}()
+
+	lockCtx, lock, err := w.acquireActorLock(ctx, actorRef)


ResumeActor is called on every event, and ActorResumer has no local "already running" cache — it always issues the ResumeActor RPC. That's exactly why ResumeState.WasRunning and the Resumed response field exist (ResumeOutcomeNone = "actor already running" in the router's own docs table).

This defer records unconditionally, so the sample rate of ate.actor.lifecycle.operation.duration{operation="resume"} equals router QPS, and the population is dominated by sub-millisecond no-op resumes that fast-forward every step. The p50/p99 you'd read off this is the warm-path no-op latency, not "when a suspended actor can serve requests again" — which is not the stated goal of the metric.

Consider adding a check here:

if err == nil && state.WasRunning { return // already running: not a resume, and the router calls this per request }

Added your guard in the latest push + a test pinning it.

Jeff Luo (JeffLuoo) · 2026-08-04T14:35:53Z

 	actorTemplateLister listersv1alpha1.ActorTemplateLister
 	workerPoolLister    listersv1alpha1.WorkerPoolLister
 	actorWorkflow       *ActorWorkflow
+	instruments         *Instruments


The instrument is threaded into two different layers:

create / delete are recorded in the gRPC handler (create_actor.go, delete_actor.go), so they include request validation, lister lookups, and the final gRPC status.

resume / suspend / pause are recorded inside ActorWorkflow (workflow.go), so they exclude the handler's validation.

Can we handle all metrics in gRPC handlers to align?

The split is intentional to some extent. The workflow owns the labels, so recording in handlers would drop those dims from exactly the failure datapoints they're for. But I agree about this not being fully consistent as validation failures currently count create/delete and not the workflow operations.

I'd rather fix that by moving the create/delete recording after validation, so all five operations measure a validated operation, and keep malformed requests visible in rpc.server.call.duration. Wdyt?

Tim Bai (baizhenyu)

Thanks for this. The WasRunning skip in the latest commit is a good fix, and reusing the existing signal means the metric and the resumed return value cannot disagree.

I have left some questions inline about the metric definitions. I would like to settle these before we build dashboards on top of them.

…ider buckets, delete template dims

Krisztian F (krisztianfekete) marked this pull request as ready for review July 24, 2026 11:06

Eitan Yarmush (EItanya) reviewed Jul 24, 2026

View reviewed changes

Krisztian F (krisztianfekete) mentioned this pull request Jul 28, 2026

Initial set of platform metrics around lifecycle, scheduling, and fleet-saturation #433

Open

Krisztian F (krisztianfekete) force-pushed the feat/add-lifecycle-and-scheudler-metrics branch 2 times, most recently from 72daaf6 to 00fb7ca Compare July 29, 2026 10:15

Krisztian F (krisztianfekete) requested a review from Eitan Yarmush (EItanya) July 29, 2026 10:26

Krisztian F (krisztianfekete) force-pushed the feat/add-lifecycle-and-scheudler-metrics branch 3 times, most recently from 26c376a to cd0945a Compare July 31, 2026 08:40

Krisztian F (krisztianfekete) added 3 commits August 3, 2026 10:09

add actor lifecycle + scheduler duration metrics

8564880

update o11y docs

e4d7b6f

address review feedback

1c136ae

Krisztian F (krisztianfekete) force-pushed the feat/add-lifecycle-and-scheudler-metrics branch from cd0945a to 1c136ae Compare August 3, 2026 08:11

Krisztian F (krisztianfekete) mentioned this pull request Aug 4, 2026

Identify what metrics we should use for HPA-based workerpool autoscaling #671

Open

Da Huang (git286) requested review from Jeff Luo (JeffLuoo) and Da Huang (git286) August 4, 2026 14:01

Jeff Luo (JeffLuoo) requested changes Aug 4, 2026

View reviewed changes

skip no-op resume metrics and align recording after validation

97b0871

Krisztian F (krisztianfekete) requested a review from Jeff Luo (JeffLuoo) August 4, 2026 15:07

Jeff Luo (JeffLuoo) previously approved these changes Aug 4, 2026

View reviewed changes

Tim Bai (baizhenyu) reviewed Aug 4, 2026

View reviewed changes

address metrics review: local snapshot kind, scheduler class label, w…

460df40

…ider buckets, delete template dims

Krisztian F (krisztianfekete) dismissed Jeff Luo (JeffLuoo)’s stale review via 460df40 August 4, 2026 15:56

Krisztian F (krisztianfekete) requested review from Jeff Luo (JeffLuoo) and Tim Bai (baizhenyu) August 4, 2026 16:01

Jeff Luo (JeffLuoo) approved these changes Aug 4, 2026

View reviewed changes

Tim Bai (baizhenyu) approved these changes Aug 4, 2026

View reviewed changes

Jeff Luo (JeffLuoo) merged commit 6f2b217 into agent-substrate:main Aug 4, 2026
11 checks passed

Krisztian F (krisztianfekete) deleted the feat/add-lifecycle-and-scheudler-metrics branch August 4, 2026 18:13

This was referenced Aug 5, 2026

feat: Add control plane telemetry - actor crashes counter and operation labels #642

Merged

[DONOTMERGE](feat): Add initial platform metrics #434

Closed

Uh oh!

Conversation

Krisztian F (krisztianfekete) commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Krisztian F (krisztianfekete) commented Jul 31, 2026

Uh oh!

Zoe Zhao (zoez7) commented Jul 31, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tim Bai (baizhenyu) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Krisztian F (krisztianfekete) commented Jul 24, 2026 •

edited

Loading